Use CMAKE_SYSROOT instead of EMSCRIPTEN_SYSROOT#27083
Conversation
|
|
||
| # You **should** use CMAKE_SYSROOT, the one below is kept for | ||
| # compat for whichever soul used that variable... | ||
| set(CMAKE_SYSROOT "${_emcache_output}/sysroot") |
There was a problem hiding this comment.
I think the reason we don't do this is that emcc on it own already automatically includes the sysroot.
i.e. you never need to do emcc --sysroot=/path/to/emscripten/sysroot since emcc already knows exactly where its sysroot is. Its not something we expect user to pass.
I think we don't really want cmake to inject and additional --sysroot since it would be redundant.
There was a problem hiding this comment.
If we set the compiler to clang then it would make sense to pass things like --sysroot and --target=wasm32-unknown-emscripten, but when we use the emcc wrapper around clang they are not needed.
There was a problem hiding this comment.
I'm aware emcc itself uses --sysroot..
HOWEVER
many projects use ${CMAKE_SYSROOT} for other miscellany purpouses, most notably:
- pkg search
- pkgconfig
- find package
- checks
- unholy usages of "absolute paths" that depend on ${CMAKE_SYSROOT}
For example shadPS4 uses CMAKE_SYSROOT for checks and stuff, is it bad design? No! As per documentation CMAKE_SYSROOT MUST point to a valid sysroot, otherwise it's not a valid toolchain.
There was a problem hiding this comment.
For find package we set CMAKE_FIND_ROOT_PATH.
For pkg_config we set PKG_CONFIG_LIBDIR.
As I said, I'm to to keem on the idea of seeing --sysroot=/path/to/default/sysroot added to every emcc command line that cmake generates.
@bradking, sorry to loop you in again here, but I'm not sure what the best approach is here. We do use a sysroot, but its location is already know the emcc. I guess I'm looking for way to tell cmake about the sysroot without injecting superfluous/redundant compiler flags. Is that something is possible? Are we wrong to not set CMAKE_SYSROOT?
There was a problem hiding this comment.
Maybe something like DEFAULT_SYSROOT which would allow cmake to be aware or it without having to also inject --sysroot?
There was a problem hiding this comment.
CMAKE_SYSROOT is meant for passing --sysroot to the compiler, and is also used for find_package and such. If one only wants the find_package part, CMAKE_FIND_ROOT_PATH should be sufficient.
There was a problem hiding this comment.
Is it really?
I'm pretty sure by this point most projects [that worry about CMAKE_SYSROOT] have used it as an ad-hoc solution to get the sysroot they're living on (for whichever reason).
There was a problem hiding this comment.
CMake documentation for CMAKE_SYSROOT states:
passed to the compiler in the --sysroot flag...also used to prefix paths searched by the
find_*commands.
If projects are referencing CMAKE_SYSROOT for their own purposes, that's their own business, and they should be able to deal with it not being set.
There was a problem hiding this comment.
Hm, Of which it says
This variable may only be set in a toolchain file specified by the CMAKE_TOOLCHAIN_FILE variable.
I mean the docs of CMake w.r.t to CMAKE_SYSROOT mostly just imply that "it's a path", and yes, used for "find_*", but, if we were left to guess what the intention was, well, many people guessed that CMAKE_SYSROOT was a safe path to use to get the sysroot of the system when cross compiling. And now it's in various projects.
Was this correct to do? Eh... well that's how it's used nowadays (aside from the use outlined on CMake itself)
Here is varying examples of CMAKE_SYSROOT, all of them using it as ""intended"" https://man.freebsd.org/cgi/man.cgi?query=cmake-toolchains&sektion=7&manpath=FreeBSD+13.2-RELEASE+and+Ports
Except, annoyingly, " Cross Compiling using Clang" which doesn't. Allegedly because it assumes the toolchain itself already sets the sysroot by itself(?). But the whole document implies that a sysroot should be used.
Also wasi does it as well https://github.com/WebAssembly/wasi-sdk/blob/5faf80805397ae2a96ab224d1f103798af06dd92/cmake/wasi-sdk-sysroot.cmake#L67
There was a problem hiding this comment.
I think with wasi-sdk the --sysroot argument is actually requires since the compiler it doesn't always know how to find it (unlike emcc).
I do understand your frustration here though, and I agree that we should somehow weight the costs and benefits here of redundantly injecting --sysroot.
No description provided.